fix(files_sharing): normalize share target on parent folder rename - #63956
fix(files_sharing): normalize share target on parent folder rename#63956silverkszlo wants to merge 2 commits into
Conversation
When a recipient moved an incoming share into one of their own folders and later renamed that folder, Updater::renameChildren passed the mount point to SharedMount::moveMount. Mount points always end in a slash, and stripUserFilesPath did not normalize its result, so the slash was stored in share.file_target. PROPFIND on such a share then returns 500. Normalize the stripped path so no caller can write a trailing slash, and repair rows that are already affected. Signed-off-by: silver <s.szmajduch@posteo.de> Assisted-by: ClaudeCode:claude-opus-5
| $query = $this->connection->getQueryBuilder(); | ||
| $query->select('id', 'file_target') | ||
| ->from('share') | ||
| ->where($query->expr()->like('file_target', $query->createNamedParameter('%/'))) |
There was a problem hiding this comment.
This does not use any index thus is extremely expensive on big instances.
Meaning we likely want to only run this once for one update and later do it not again on the next update.
There was a problem hiding this comment.
Maybe since this is needed to fix an issue and we need to backport it down to the first version with authoritative mount points, it probably makes sense to instead make a new repair step and register it as expensive, so users get the hint that they should run it when they can, rather than cramping it during the update. WDYT?
There was a problem hiding this comment.
Yes I think two options:
- Make it its own repair step (expensive repair step) -> allow manually re-triggering
- Keep in this repair step but add appconfig flag "has this been done already" and if yes do not run again
|
Tested on master, fixes the issue so besides the expensive repair step running at every upgrade, it looks good to me. |
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
4 participants
Summary
When a recipient moved an incoming share into one of their own folders and later renamed that folder,
Updater::renameChildrenpassed the mount point toSharedMount::moveMount. Mount points always end in a slash, andstripUserFilesPathdid not normalize its result, so the slash was stored inshare.file_target.PROPFINDon such a share then returns500.Normalize the stripped path so no caller can write a trailing slash, and repair rows that are already affected.
To reproduce
occ share:list --recipient=[recipient]→ The name of the share has a trailing slash in
target-path:/box_renamed/test.odt/Checklist
3. to review, feature component)stable32)AI (if applicable)